feat: add configuration options to support mtls #315

coryb · 2023-10-22T23:51:27Z

adding options to support mtls with the coder server. This supports adding PEM certs and keys to the tls requests, and also supports adding a CA cert to the trust store. Also allowing for an alternate hostname that may appear in the certs which is useful for testing or for non-standard cert usage.

This is similar to the change for the vscode plugin: coder/vscode-coder#126

I am not terribly familiar with kotlin or java so let me know if there are non-idiomatic things you want changed.

src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt

adding options to support mtls with the coder server. This supports adding PEM certs and keys to the tls requests, and also supports adding a CA cert to the trust store. Also allowing for an alternate hostname that may appear in the certs which is useful for testing or for non-standard cert usage.

coryb · 2023-10-23T21:29:52Z

gradle.properties

@@ -29,4 +29,4 @@ gradleVersion=7.4
 # Opt-out flag for bundling Kotlin standard library.
 # See https://plugins.jetbrains.com/docs/intellij/kotlin.html#kotlin-standard-library for details.
 # suppress inspection "UnusedProperty"
-kotlin.stdlib.default.dependency=false
+kotlin.stdlib.default.dependency=true


It seems like we need the kotlin stdlib, without this I was getting:

java.lang.ClassNotFoundException: kotlin.enums.EnumEntries

I suspect this is required since the kotlin 9.x upgrade?

Where do you see this error? I tried building with this set to false and running the resulting plugin in Gateway (2023.2.4) but so far all seems well.

I see when running ./gradlew build --info or equivalent command via IntelliJ. Strange that you don't see it also 🤔

code-asher

Fabulous, code looks good to me, keeping in mind that I also lack Kotlin/Java expertise so one might say this is the blind leading the blind. 😆

The way to do this in Kotlin is absolutely wild compared to Node! Honestly I mostly glazed over that part, but I would like to read through it more carefully when I have more time.

I only did some very surface-level testing for regressions but have not had a change to try the new settings yet; I would feel more comfortable if we threw in some unit tests proving the behavior but one must opt into the changes by filling out the settings anyway so I think it is reasonable to merge this in and get a release out so we can start trying it out. I left some minor comments but I am going to have very spotty availability starting tomorrow for a couple weeks which is why I am thinking we rush a bit to publish.

Edit: heading out but will merge/release when I get back tonight.

code-asher · 2023-10-26T22:10:11Z

gradle.properties

@@ -29,4 +29,4 @@ gradleVersion=7.4
 # Opt-out flag for bundling Kotlin standard library.
 # See https://plugins.jetbrains.com/docs/intellij/kotlin.html#kotlin-standard-library for details.
 # suppress inspection "UnusedProperty"
-kotlin.stdlib.default.dependency=false
+kotlin.stdlib.default.dependency=true


Where do you see this error? I tried building with this set to false and running the resulting plugin in Gateway (2023.2.4) but so far all seems well.

code-asher · 2023-10-26T22:12:26Z

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

@@ -140,7 +140,7 @@ class CoderGatewayConnectionProvider : GatewayConnectionProvider {
        if (token == null) { // User aborted.
            throw IllegalArgumentException("Unable to connect to $deploymentURL, $TOKEN is missing")
        }
-        val client = CoderRestClient(deploymentURL, token.first, settings.headerCommand, null)
+        val client = CoderRestClient(deploymentURL, token.first,null, settings)


Suggested change

val client = CoderRestClient(deploymentURL, token.first,null, settings)

val client = CoderRestClient(deploymentURL, token.first, null, settings)

code-asher · 2023-10-26T22:16:32Z

src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt

+
+    val sslContext = SSLContext.getInstance("TLS")
+
+    val trustManagers = coderTrustManagers(settings.tlsCAPath)


Is it a problem that we only use tlsCAPath if we did not return early above when tlsCertPath or tlsKeyPath are blank? Thinking of a use case were someone only sets tlsCAPath but not the others. I see we separately call coderTrustManagers() in the rest client so maybe it works there, but possibly the binary download could have issues since we are only using the socket factory there.

code-asher · 2023-10-26T22:20:22Z

src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt

+class CoderHostnameVerifier(private val alternateName: String) : HostnameVerifier {
+    override fun verify(host: String, session: SSLSession): Boolean {
+        if (alternateName.isEmpty()) {
+            println("using default hostname verifier, alternateName is  empty")


We should copy the logger pattern used elsewhere instead of using println directly.

code-asher · 2023-10-26T22:21:16Z

src/main/kotlin/com/coder/gateway/views/CoderGatewayRecentWorkspaceConnectionsView.kt

@@ -256,7 +256,7 @@ class CoderGatewayRecentWorkspaceConnectionsView(private val setContentCallback:
                deployments[dir] ?: try {
                    val url = Path.of(dir).resolve("url").readText()
                    val token = Path.of(dir).resolve("session").readText()
-                    DeploymentInfo(CoderRestClient(url.toURL(), token, settings.headerCommand, null))
+                    DeploymentInfo(CoderRestClient(url.toURL(), token,null, settings))


Suggested change

DeploymentInfo(CoderRestClient(url.toURL(), token,null, settings))

DeploymentInfo(CoderRestClient(url.toURL(), token, null, settings))

code-asher · 2023-10-26T22:39:40Z

src/main/resources/messages/CoderGatewayBundle.properties

@@ -93,3 +93,21 @@ gateway.connector.settings.header-command.comment=An external command that \
  outputs additional HTTP headers added to all requests. The command must \
  output each header as `key=value` on its own line. The following \
  environment variables will be available to the process: CODER_URL.
+gateway.connector.settings.tls-cert-path.title=Cert Path:


Not really important, but the other settings are Capitalized like this: so changing these to Cert path: etc might be good for a more consistent look (or change the capitalization of the others).

code-asher · 2023-10-27T05:03:21Z

src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt

+    )
+
+    var privateKey : PrivateKey
+    try {


Forgot to mention you can do val privateKey = try which is pretty neat.

coryb commented Oct 23, 2023

View reviewed changes

src/main/kotlin/com/coder/gateway/sdk/CoderRestClientService.kt Outdated Show resolved Hide resolved

coryb force-pushed the tls-properties branch from 8a27010 to 2721631 Compare October 23, 2023 21:28

coryb commented Oct 23, 2023

View reviewed changes

code-asher approved these changes Oct 26, 2023

View reviewed changes

code-asher reviewed Oct 27, 2023

View reviewed changes

code-asher merged commit ac003ce into coder:main Oct 27, 2023

coryb mentioned this pull request Oct 30, 2023

chore: minor cleanup PR#315 #322

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add configuration options to support mtls #315

feat: add configuration options to support mtls #315

coryb commented Oct 22, 2023

coryb Oct 23, 2023

code-asher Oct 26, 2023

coryb Oct 29, 2023

code-asher left a comment •

edited

Loading

code-asher Oct 26, 2023

code-asher Oct 26, 2023

code-asher Oct 26, 2023 •

edited

Loading

code-asher Oct 26, 2023

code-asher Oct 26, 2023

code-asher Oct 26, 2023

code-asher Oct 27, 2023 •

edited

Loading

	val client = CoderRestClient(deploymentURL, token.first,null, settings)
	val client = CoderRestClient(deploymentURL, token.first, null, settings)


		val sslContext = SSLContext.getInstance("TLS")

		val trustManagers = coderTrustManagers(settings.tlsCAPath)

	DeploymentInfo(CoderRestClient(url.toURL(), token,null, settings))
	DeploymentInfo(CoderRestClient(url.toURL(), token, null, settings))

feat: add configuration options to support mtls #315

feat: add configuration options to support mtls #315

Conversation

coryb commented Oct 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

code-asher left a comment •

edited

Loading

code-asher Oct 26, 2023 •

edited

Loading

code-asher Oct 27, 2023 •

edited

Loading